Handle "internal" directory visibility#274
Conversation
|
@jayconrod and @segiddins, could I have you both take a look at this one also. I believe this addresses the last issue that @segiddins ran into when using this in rules_swift. If that's not the case, please help me see the things that I am missing so I can make this as easy to use as possible. Thanks so much! Once you two LGTM I will send off to someone from the skylib team for a last review. |
jayconrod
left a comment
jayconrod
left a comment
There was a problem hiding this comment.
Looks good with a couple minor comments.
gazelle/bzl/gazelle.go
Outdated
| // so it can't be used verbatim, but with slight modifications it can be. | ||
| // https://github.com/bazelbuild/bazel-gazelle/blob/2078a44d2bba287db98b2b48b5f6cf4f08642489/rule/rule.go#L833:6 | ||
| func checkInternalVisibility(rel, visibility string) string { | ||
| rel = rel + "/" // Append a / to make finding the suffix easier. |
There was a problem hiding this comment.
Maybe use github.com/bazelbuild/bazel-gazelle/pathtools.Index instead? It's like strings.Index, but it only matches on path component boundaries.
There was a problem hiding this comment.
I made the change and it is passing tests so I believe it is equivalent. The only reason I didn't use pathtools.Index instead from the start is because it wasn't looking for the LastIndex and I didn't want it to turn /foo/internal/bar/internal into being visible to //foo:__subpackages__
|
@jayconrod with the submission of the Thanks again! |
|
@jayconrod could you try approving this one more time now that we've updated the CODEOWNERS? |
|
Actually, a couple updates are needed.
|
|
@c-parsons would it be possible to add writer permissions to either (or both) me and @jayconrod |
|
@achew22 you have already had write permissions for the github repo. I've added @jayconrod now. |
Both "internal" and "private" directories should be treated the same way and have visibility restrictions be placed upon them.
5 participants
Both "internal" and "private" directories should be treated the same way
and have visibility restrictions be placed upon them.